Feature: Support stlink v2 isol variant #1720
Conversation
dragonmux
added
the
Foreign Host Board
There was a problem hiding this comment.
We've got two queries that could do with being answered, and then this looks like it's good to merge.
Please fix your commit message's prefix to conform to the contrib guidelines - in this case, it should be prefixed stlink: as that's the platform all these changes are in.
There was a problem hiding this comment.
There are macro naming questions from me and a proposal to bring feature parity with SWCLK Hi-Z, which leads into considerations about drive contention somewhere on the PCB. I cannot "Review -1/+1" as I am not a maintainer, and I am very unlikely to get my hands on an ISOL, but you could test anything before the PR is merged, and other less knowledgable users started (hypothetical worst case scenario) damaging their boards.
I assume you already live-tested your PR branch build on a unit?
| /* PB12 is SWDIO_IN */ | ||
| gpio_set_mode(GPIOB, GPIO_MODE_INPUT, GPIO_CNF_INPUT_FLOAT, GPIO12); | ||
| /* PA4 is used to set SWDCLK floating when set to 1 */ | ||
| gpio_set_mode(GPIOA, GPIO_MODE_OUTPUT_2_MHZ, GPIO_CNF_OUTPUT_PUSHPULL, GPIO4); | ||
| gpio_clear(GPIOA, GPIO4); | ||
| /* PA1 is used to set SWDIO floating and MUXED to SWDIO_IN when set to 1 */ | ||
| gpio_set_mode(GPIOA, GPIO_MODE_OUTPUT_2_MHZ, GPIO_CNF_OUTPUT_PUSHPULL, GPIO1); |
There was a problem hiding this comment.
If you wanted to, you could implement feature parity with BMP v2.3 HW rev 6 w.r.t. floating the SWCLK pin, required to support stm32g0 in small pin-count packages. See Issue 945 and PR1192 (intentional not-a-mention), specifically commit a573c26 for native platform.
PB12 is T_SWDIO_IN on most/all stlinkv2 schematics, and you made it a macro -- please substitute it into here (SWDIO_IN_PIN).
Accordingly, PA4 sounds like TCK_DIR_PIN and PA1 like TMS_DIR_PIN/SWDIO_DIR_PIN. This makes for less unused/magic pin numbers, even if you described their purpose in adjacent comments (nice!).
| #ifdef STLINK_V2_ISOL | ||
| /* In case of ISOL variant, this pin is never set to high impedance */ | ||
| gpio_set_mode(TMS_PORT, GPIO_MODE_OUTPUT_2_MHZ, GPIO_CNF_OUTPUT_PUSHPULL, TMS_PIN); |
There was a problem hiding this comment.
I wonder if that's true -- PB14 being T_JTMS (effectively T_SWDIO_OUT) was toggled to become input across BMF previously. Do you think PB14 forcing high/low SWDIO in no way interferes with receiving data on PB12 across the isolation circuitry? Although I don't have the exact schematics and don't know where the 100R between SWDIO_IN & SWDIO_OUT resides for ISOL PCB. But you seem to have the means to test it with live targets.
HrMitrev
force-pushed
the
feature/support-stlink-v2-isol
branch
from
0d476c2 to
b85f431
Compare
I have unpacked/inlined the mode switch of SWDIO and confirmed to be working on my isol probe. Also I have rebased. Currently I have used this patch for more than 2 weeks every day and my probe seems to be going strong. I haven't had the chanse to confirm it is working with JTAG instead of SWD. In the future if I have more time and HW to test JTAG I will revisit. For now I think this can be merged Cheers! |
This was hidden after the changes, but may be a good idea to be easily accessible Also it may be useful to provide the build command used - |
Detailed description
The stlink-isol-v2 variant differs from other stlink-v2 variants with how the SWDIO pin is multiplexed. This commit adds support for it. I have tested the commit on my STlink ISOL and it seems to work fine.
Your checklist for this pull request
make PROBE_HOST=native)make PROBE_HOST=hosted)Closing issues
N/A